Make the revision part percent-encoded in version control URLs #13407
Conversation
|
Attempting to fix #10226 |
|
We need to add test cases. |
Added. |
|
Thanks for the PR, it may take some time to review as maintainers capacity is very limited. A couple of quick questions:
|
The only case usage will change is when any revision or branch name has % characters, and those percents will have to be encoded as %25 then, although I don't know if it worked without the encoding previously. Users must modify I will add a simple example later. Thanks. |
We should call this out in the |
|
Updated the news entry with more details. |
|
@uranusjr since you were quite involved in the discussions on the associated issue, could you take a look at this PR? TIA! |
There was a problem hiding this comment.
Pull Request Overview
This PR implements percent-encoding for revision specifications in version control URLs to properly handle branch names containing special characters like '@' and '#'. When these characters appear in branch names, they need to be URL-encoded to avoid conflicts with URL parsing.
- Adds percent-encoding of revision strings when constructing VCS URLs
- Adds percent-decoding of revision strings when parsing VCS URLs
- Includes comprehensive test coverage for the new encoding/decoding functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/pip/_internal/vcs/versioncontrol.py | Implements percent-encoding/decoding for revision parts in VCS URLs |
| tests/unit/test_vcs.py | Adds test cases to verify proper handling of special characters in revisions |
| news/13407.bugfix.rst | Documents the bugfix and breaking change for users with '%' in branch names |
|
@uranusjr ping again, would you be interested in reviewing this PR? If not, I'll add this to my list of things to review (although it's low priority and out of my wheelhouse so it'll take a while). |
There was a problem hiding this comment.
Ideally we’d want to do this in a backward compatible way, but I don’t think that’s very worthwhile given the likely limited blast radius. Hopefully nobody has both branches with and without percent-escaping names that could cause an unnoticed behavioural change. (Otherwise the most offending issue would be the command start throwing an error, which is hopefully a one-off issue to fix.)
I wonder if this should also be called out as a .feature change and/or a .process one instead.
4 participants
This PR changes how the revision specification is handled in version control URLs. When a branch name contains special characters such as '@' or '#', they must be percent-encoded in URLs; however, this is not currently implemented.
Fixes #10226